Generated by 🚫 dangerJS against 1d65c27

Base commit: ae07c53

Base commit: ae07c53

@hramos The CI is passed now for this PR. Could you please take a look? Hopefully this could be landed and have a more stabler JSC in the future.

@hramos, please review this PR, @Kudo thanks for the contribution to fix this

Here I'm pasting the outcome from our quick investigation over this PR:

@Kudo Please review ..

A quick evaluation of this PR which is upgrading the JavaScriptCore binaries used by react-native on Android platform,

The PR is upgrading JSC binaries from revision 245459 to 250230. These numbers are the SVN revision numbers in Webkit repo (svn info "${URL}" | sed -n 's/^Last Changed Rev: //p') akin to the commit ids in Git repos.

These binaries are generated based on the WebKitGtk sources and the build scripts are stored in this repo :react-native-community/jsc-android-buildscripts: Script for building JavaScriptCore for Android (for React Native but not only) (github.com)

The current JSC package consumed in react-native

The last JSC upgrade in react-native was through this commit: https://github.com/facebook/react-native/commit/58d87940a1a995f0faf1270c0da435b44015575f on Jul 1, 2019.

It was also built by Kudo

This was the commit description,

Upgrade jsc-android to r245459 and fix crash on Samsung S7 Edge (#25426)

Summary:

Upgrade bundled jsc-android that should fix the native JSC crash addressed for #24261.

Major changes after r241213 are:

1. Disable DFG JIT.

2. Upgrade WebKitGTK to 2.24.2, this version includes the [new bytecode format enhancement](https://webkit.org/blog/9329/a-new-bytecode-format-for-javascriptcore/).

3. Workaround LLVM \_\_clear\_cache issue which may have some problems on ARM Cortex A-53.

For details, please refer to the PRs in https://github.com/react-native-community/jsc-android-buildscripts.

In #24261, there were many experimented JSC deliveries and thank to people in RN community helping me verify the final version which solve the crash issues.

Even though the above commit description suggested that the JIT was being turned off, by looking at the changes that went into the JSC build scripts, the JIT was turned back on based on the feedbacks on adverse impact on performance,

react-native-community/jsc-android-buildscripts@abdce96

commited on Jun 25, 2019

Here is a summary on the state of JSC package at present,

1. Based in WebKitGtk release v2.24.2 (SVN revision 245459) released on 05/17/2019
2. ICU 58.1
3. Based on Android ndk version r17c

Latest PR

By looking at the changes went into the build scripts, the following are the enhancements in the JSC binaries with the latest PR,

1. Based on WebKitGTK 2.26.1 (SVN revision 250230) dated on 09/23/2019
2. Upgraded ICU from v58.1 to v64.2
3. Built with Android NDK r19c
4. One key crash fix is included,

react-native-community/jsc-android-buildscripts@57a2aa0

Summary

I'd summarize my thoughts below,

1. he new package is also based on a 1½ year old version of Webkit source and is only 4 months ahead of the current one. Hence I don’t expect the new binaries to be much different, and any important security fixes to be included.
2. The crash fix mentioned above seems to be one that community is looking forward to. The fix is not yet tested and verified to fix the crash though as far as I understood. But the crash seems to be happening because of synchronization issues between GC and interpreter access to bytecode pages and the fix is addressing it, hence it will likely fix the crash.
3. ICU bump from 58.* to 64.* can mean a lot for products which consumes JSC with Intl enabled.

@yungsters has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@yungsters merged this pull request in 341f061.

Thanks @mganandraj for the briefly description which is correct.
and thanks @yungsters for getting this landed.

Sure thing. Thanks right back at you, @Kudo! And yes, much appreciation to @mganandraj for the analysis that helped me do a lot of the legwork to gain the context for merging this.